Skip to content

Conversation

@mitchell-as
Copy link
Collaborator

@mitchell-as mitchell-as commented Nov 8, 2024

BugDX-3152 "Target & Verify" automation removes an existing "Found_By_QA_Automation" label

@mitchell-as
Copy link
Collaborator Author

Tested by adding a label to the JIRA issue, then running Target & Verify, and checking that "WasNextFeasible" was appended instead of overwriting the existing label.

@mitchell-as mitchell-as requested a review from Naatan November 8, 2024 20:27
@mitchell-as mitchell-as marked this pull request as ready for review November 8, 2024 20:28
@mitchell-as
Copy link
Collaborator Author

image

@mitchell-as
Copy link
Collaborator Author

@Naatan ping

Copy link
Contributor

@Naatan Naatan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it would only update based on the fields provided. If it empties anything that's not provided then this may also affect other fields. Would perhaps a more robust fix be to assign all fields, and then update FixVersions? Right now it seems like anything but FixVersions and Labels might get pruned.

@mitchell-as
Copy link
Collaborator Author

The concern you raised may look problematic based on the diff GitHub shows, but the troublesome line is here: https://github.com/ActiveState/cli/pull/3588/files#diff-2224e300e5d1da6fa67f6cf4cfc1d6b288f60bda27d6d0181bce500076850273R192. We need to initialize the struct with the current set of labels if we want to append to it. Otherwise, we're appending to an empty list, which will overwrite the current label.

I think we're fine.

@mitchell-as mitchell-as requested a review from Naatan November 12, 2024 20:36
@Naatan
Copy link
Contributor

Naatan commented Nov 12, 2024

The concern you raised may look problematic based on the diff GitHub shows, but the troublesome line is here: #3588 (files). We need to initialize the struct with the current set of labels if we want to append to it. Otherwise, we're appending to an empty list, which will overwrite the current label.

I think we're fine.

Ahh ok, that makes sense. Thanks for explaining!

@mitchell-as mitchell-as merged commit 48b1d05 into version/0-48-0-RC1 Nov 12, 2024
@mitchell-as mitchell-as deleted the mitchell/dx-3152 branch November 12, 2024 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants